Skip to content

grwehner/mdm custom metric regions#471

Merged
gracewehner merged 6 commits into
ci_devfrom
grwehner/mdm-custom-metric-regions-and-msi-start-time
Nov 23, 2020
Merged

grwehner/mdm custom metric regions#471
gracewehner merged 6 commits into
ci_devfrom
grwehner/mdm-custom-metric-regions-and-msi-start-time

Conversation

@gracewehner

@gracewehner gracewehner commented Nov 11, 2020

Copy link
Copy Markdown
Contributor
  • Remove custom metrics region check for public cloud by removing the custom_metrics_azure_regions variable for plugins and changing to only check the cloud environment. This is set as an environment variable using the workspace domain in the main.sh file so that it'll work with AKS and ARC.

My questions:
Should the check be more explicit by specifically checking if the workspace domain is indeed for a national cloud? Should custom metrics be available for any other cluster with a workspace domain that isn't opinsights.azure.com? What should the strategy be if the domain is not set?

@gracewehner gracewehner requested a review from a team November 11, 2020 19:11
vishiy
vishiy previously approved these changes Nov 11, 2020
@gracewehner gracewehner changed the title grwehner/mdm custom metric regions and msi start time grwehner/mdm custom metric regions Nov 21, 2020
@gracewehner gracewehner requested a review from a team November 21, 2020 01:30
Comment thread kubernetes/linux/main.sh
# Set environment variable for if public cloud by checking the workspace domain.
if [ -z $domain ]; then
ClOUD_ENVIRONMENT="unknown"
elif [ $domain == "opinsights.azure.com" ]; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think as we are starting to add more constants here, we can move the string constants to a file. Not in the scope of this PR. But we can discuss and make the changes later.

@gracewehner gracewehner merged commit 18c27dd into ci_dev Nov 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants